Skip to content

Fix log level: CLI phase suppression, case-insensitive "default" key, dynamic runtime log level#3235

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/implement-fixes-and-refactor
Draft

Fix log level: CLI phase suppression, case-insensitive "default" key, dynamic runtime log level#3235
Copilot wants to merge 3 commits intomainfrom
copilot/implement-fixes-and-refactor

Conversation

Copy link
Contributor

Copilot AI commented Mar 12, 2026 •
edited
Loading

Why make this change?

Implements and refines the log level improvements from PR #3203, addressing all open review comments:

  1. dab start --LogLevel Warning still emitted CLI-phase Information messages because CustomConsoleLogger had a hardcoded LogLevel.Information minimum and the logger factory was created before CLI args were parsed.
  2. Config-defined log level key "Default" (capital D) was silently ignored — TryGetValue("default") is case-sensitive.
  3. Log filter segment comparison in IsLoggerFilterValid was case-sensitive, rejecting valid mixed-case namespace filters.
  4. No mechanism to defer log output until after the runtime config's log level was known, causing early startup messages to bypass the configured level.

What is this change?

New: DynamicLogLevelProvider (src/Service/Telemetry/)

  • Singleton holding the current minimum log level; initialised from CLI args, then optionally updated from RuntimeConfig unless the CLI already overrode it.
  • Log filters in Program.GetLoggerFactoryForLogLevel and ConfigureLogging delegate to LogLevelProvider.ShouldLog() instead of capturing a static level at startup.

New: StartupLogBuffer (src/Config/)

  • Thread-safe queue that captures log entries before the DI container has a real ILogger.
  • FileSystemRuntimeConfigLoader writes to the buffer during early startup; Startup.Configure calls FlushLogBuffer() once the proper logger is resolved.

CLI phase respects --LogLevel

  • Cli/Program.Main calls new PreParseLogLevel(args) before CommandLine.Parser runs, wiring the result into CustomLoggerProvider(minimumLogLevel).
  • CustomConsoleLogger._minimumLogLevel is now constructor-injected instead of hardcoded to Information.

Bug fixes

  • RuntimeConfig.GetConfiguredLogLevel: StartsWith → OrdinalIgnoreCase; "default" key lookup uses FirstOrDefault with OrdinalIgnoreCase (was TryGetValue("default"), exact match only).
  • RuntimeConfigValidator.IsLoggerFilterValid: segment comparison uses OrdinalIgnoreCase.
  • FileSystemRuntimeConfigLoader: all Console.WriteLine calls replaced with private LogInformation/LogWarning/LogError helpers that route _logger → buffer → Console; eliminated duplicate log entry in hot-reload path.
  • Program.StartEngine: removed Console.WriteLine("Starting the runtime engine...") (suppressed at Warning+ but was checked by tests regardless).

Test refactoring

  • TestEngineStartUpWithVerboseAndLogLevelOptions split into two [DataTestMethod] groups:
    • Trace/Debug/Information: asserts CLI output lines are present.
    • Warning/Error/Critical/None: uses process.WaitForExit(1000) + Assert.IsFalse(exitedWithinTimeout) — fixes the race condition from Assert.IsFalse(process.HasExited).
  • ValidStringLogLevelFilters gains a "Default" (capital D) data row.

How was this tested?

  • Integration Tests
  • Unit Tests
    • StartupLogBufferTests (4): replay order, null-logger discard, idempotent flush, empty buffer
    • DynamicLogLevelProviderTests (6): SetInitialLogLevel, UpdateFromRuntimeConfig with/without CLI override, ShouldLog boundary cases
    • GetConfiguredLogLevelTests (3): "default" / "Default" / "DEFAULT" variants, specific-filter-over-default priority
    • ProgramPreParseLogLevelTests (22): all numeric and named log levels including mixed-case, edge cases (absent flag, invalid value, out-of-range, flag-with-no-value)

Sample Request(s)

# CLI-phase messages now suppressed when --LogLevel Warning is passed
dab start --config dab-config.json --LogLevel Warning
# No "Information: Microsoft.DataApiBuilder v..." output

# Config file with title-case "Default" key now works correctly
# runtime.telemetry.log-level: { "Default": "Warning" }
dab start --config dab-config.json
Original prompt

review this PR. also check the open review comments if any. consolidate with your review comments and generate the details here. implement the fixes here. ensure test case coverage. the functionality should work end to end. refactor the code and keep it modular including test cases by merging similar tests into datatestmethod.

The user has attached the following file paths as relevant context:

  • .github\copilot-instructions.md

Created from VS Code.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 2 commits March 12, 2026 09:31
@souvikghosh04
Implement LogLevel fixes: DynamicLogLevelProvider, StartupLogBuffer, …
b27e224 
…case-insensitive default key, CLI log level pre-parsing

Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>
@souvikghosh04
Address code review comments: remove redundant filter, fix duplicate …
6e377b8 
…log, use FirstOrDefault for default key

Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor code and ensure test case coverage Fix log level: CLI phase suppression, case-insensitive "default" key, dynamic runtime log level Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aniruddh25 Aniruddh25 Awaiting requested review from Aniruddh25 Aniruddh25 will be requested when the pull request is marked ready for review Aniruddh25 is a code owner

@aaronburtle aaronburtle Awaiting requested review from aaronburtle aaronburtle will be requested when the pull request is marked ready for review aaronburtle is a code owner

@anushakolan anushakolan Awaiting requested review from anushakolan anushakolan will be requested when the pull request is marked ready for review anushakolan is a code owner

@JerryNixon JerryNixon Awaiting requested review from JerryNixon JerryNixon will be requested when the pull request is marked ready for review JerryNixon is a code owner

@RubenCerna2079 RubenCerna2079 Awaiting requested review from RubenCerna2079 RubenCerna2079 will be requested when the pull request is marked ready for review RubenCerna2079 is a code owner

@souvikghosh04 souvikghosh04 Awaiting requested review from souvikghosh04 souvikghosh04 will be requested when the pull request is marked ready for review souvikghosh04 is a code owner

@sourabh1007 sourabh1007 Awaiting requested review from sourabh1007 sourabh1007 will be requested when the pull request is marked ready for review sourabh1007 is a code owner

@vadeveka vadeveka Awaiting requested review from vadeveka vadeveka will be requested when the pull request is marked ready for review vadeveka is a code owner

@Alekhya-Polavarapu Alekhya-Polavarapu Awaiting requested review from Alekhya-Polavarapu Alekhya-Polavarapu will be requested when the pull request is marked ready for review Alekhya-Polavarapu is a code owner

@rusamant rusamant Awaiting requested review from rusamant rusamant will be requested when the pull request is marked ready for review rusamant is a code owner

@stuartpa stuartpa Awaiting requested review from stuartpa stuartpa will be requested when the pull request is marked ready for review stuartpa is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

Copilot code review Copilot

@souvikghosh04 souvikghosh04

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@souvikghosh04